-
Notifications
You must be signed in to change notification settings - Fork 8k
boards: silabs: Add xg27 radio boards #96296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
boards: silabs: Add xg27 radio boards #96296
Conversation
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
board_runner_args(jlink "--device=EFR32MG27CxxxF768") | ||
else() | ||
board_runner_args(jlink "--device=EFR32BG27CxxxF768") | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe:
string(TOUPPER ${CONFIG_SOC} JLINK_DEVICE)
string(SUBSTRING 0 17 ${JLINK_DEVICE} JLINK_DEVICE)
board_runner_args(jlink "--device=${JLINK_DEVICE}")
... can be done in another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't have xxx
in the middle...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed (it seems the full chip name is sometime accepted by JLink, but not always). We probably need some more robust:
string(REGEX REPLACE "^(EF[RM]32[PFBM]G2[1-9][ABC])[0-9]*(F[0-9]*).*" "\\1xxx\\2" JLINK_DEVICE "${JLINK_DEVICE}")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the risk to have a future rb4119b that does not match with this definition? That's said, I have not better proposal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very low. But if it happens, it doesn't need to include the generic .dtsi file.
949a94a
to
c085794
Compare
default "efr32bg27c230f768im32" if SOC_EFR32BG27C230F768IM32 | ||
default "efr32bg27c230f768im40" if SOC_EFR32BG27C230F768IM40 | ||
default "efr32bg27c320f768gj39" if SOC_EFR32BG27C320F768GJ39 | ||
default "efr32bg27c320f768ij39" if SOC_EFR32BG27C320F768IJ39 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you miss the dtsi for this one ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch.
c085794
to
a2f4154
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Please update west.yml
now that the corresponding module PR was merged.
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
if(CONFIG_BOARD_XG27_RB4194A) | ||
board_runner_args(jlink "--device=EFR32MG27CxxxF768") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 space indent for cmake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks!
model = "Silicon Labs BG27-RB4110B"; | ||
compatible = "silabs,bg27_rb4110b", "silabs,efr32bg27"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be referring to bg27_rb4111b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks!
compatible = "gpio-keys"; | ||
|
||
button0: button_0 { | ||
gpios = <&gpiob 2 GPIO_ACTIVE_LOW>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
led0 and button0 on the same pin? probably a typo somewhere :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. Then you'll want to disable either buttons or led node initially, or people trying to run the button
sample are probably in for a surprise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. Disabling the LED node in board dts will prevent the blinky sample from working, so that's not a great solution. Maybe just removing the led0
alias in a board overlay for the button sample is the most sensible approach? Could also swap 0 and 1 around, but then the dts nodes don't match the silkscreen on the board...
4b466f8
4b466f8
to
997d795
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rebase is needed
a7b54e5
to
8507332
Compare
Set #address-cells and #size-cells on usart0. Since usarts can be used for both UART and SPI, this property shouldn't be set in SoC-level DTS, since it makes it harder to switch compatible. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>
Reformat devicetree files: * Sort nodes by unit address or name * Sort properties by category and name Add missing properties to existing nodes. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>
Add missing peripheral and clock nodes for xg27. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>
Add efr32bg27 and efr32mg27 socs. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>
Use cryptoacc variant on all xg27 socs. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>
Add 3 xg27 radio boards: * bg27_rb4110b * bg27_rb4111b * xg27_rb4194a Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>
Button 0 and LED 0 share a GPIO pin on bg27 boards. Remove the `led0` alias in a board overlay to make the button sample work out of the box. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>
8507332
to
81f94f1
Compare
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are probably a few other samples that might have incorrect out-of-the-box experience due to button/led overlap but this is probably fine given that the caveat is called out in the board's documentation
Reformat xg27 soc devicetree files according to https://docs.zephyrproject.org/latest/contribute/style/devicetree.html and https://docs.kernel.org/devicetree/bindings/dts-coding-style.html:
Add support for all socs in the xg27 family.
Add three xg27 radio boards:
bg27_rb4110b
,bg27_rb4111b
andxg27_rb4194a
.